Skip to content

Fix checkJs behavior for .mjs and .cjs files next to declarations - #64527

Open
Hardik K (hardikkaurani) wants to merge 4 commits into
microsoft:mainfrom
hardikkaurani:fix/64312
Open

Hardik K (hardikkaurani) wants to merge 4 commits into
microsoft:mainfrom
hardikkaurani:fix/64312

Conversation

@hardikkaurani

@hardikkaurani Hardik K (hardikkaurani) commented Sep 29, 2026 •

Copy link
Copy Markdown

Currently, when allowJs: true and checkJs: true are enabled, .js files located next to .d.ts files are included for type checking due to a legacy exception in the extension priority rules. However, .mjs and .cjs files alongside their .d.mts and .d.cts counterparts were skipped.

This PR aligns the behavior of .mjs and .cjs files with .js files by extending the legacy exemption in hasFileWithHigherPriorityExtension and removeWildcardFilesWithLowerPriorityExtension. This ensures consistent behavior across all JS extensions, as requested by users.

Fixes #64312

@typescript-automation typescript-automation Bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Sep 29, 2026
@typescript-automation

Copy link
Copy Markdown
Contributor

This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise.

@hkleungai

Copy link
Copy Markdown

Looks like this PR has a chance to resolve #63523 too. 😀

@hardikkaurani

Copy link
Copy Markdown
Author

Jimmy Leung (@hkleungai) Great catch! Yes, this PR should indeed fix the inconsistency reported in #63523. By adding the identical exemption to
emoveWildcardFilesWithLowerPriorityExtension, it ensures that the file inclusion is order-independent, fixing the reorder hack you noticed.

Ryan Cavanaugh (@RyanCavanaugh) Just a heads up! I've linked this PR to #64312 to ensure consistent behavior for .mjs/.d.mts\ and .cjs/.d.cts\ files, mirroring the existing .js/.d.ts\ legacy behavior. As an added benefit, this also patches the wildcard parsing order dependency noted in #63523, making the behavior deterministic regardless of the \include\ array order. Let me know if you have any questions or feedback!

@jakebailey

Copy link
Copy Markdown
Member

No tests?

@hardikkaurani

Hardik K (hardikkaurani) commented Sep 29, 2026 •

Copy link
Copy Markdown
Author

Jake Bailey (@jakebailey) Thanks for the feedback!

I've added a regression test in tsc/testdata/tests/cases/compiler/checkJsExtensionPriority.ts that reproduces the original issue by exercising wildcard discovery for .mjs and .cjs files alongside their declaration counterparts with checkJs: true. The generated baselines confirm that these files are now correctly included in the compilation and checked for type errors.

@weswigham

Copy link
Copy Markdown
Member

...Why would we make the new extensions have an explicitly legacy behavior?

@weswigham

Copy link
Copy Markdown
Member

If anything, we should remove the legacy lookup behavior from .js, no? Seems unlikely at this point that anyone intentionally includes a .js and .d.ts file referring to the same input.

@hardikkaurani

Hardik K (hardikkaurani) commented Sep 29, 2026 •

Copy link
Copy Markdown
Author

Wesley Wigham (@weswigham) I looked further into the existing extension-priority behavior and its history before making any changes.

The .js/.d.ts coexistence is explicitly documented in the compiler as legacy behavior originating from an off-by-one issue, with the code comment noting that it is generally undesirable but retained for compatibility. I also checked #63523 and #47796, which reinforce the compiler's general behavior of selecting the higher-priority file for a given basename rather than intentionally ingesting both.

Given that, extending the existing .js/.d.ts legacy exception to .mjs/.d.mts and .cjs/.d.cts would carry that compatibility behavior into the newer module formats.

For #64312, the issue explicitly prioritizes consistency over which behavior is chosen. One consistent approach would therefore be to remove the legacy .js/.d.ts exception and have all three extension pairs follow the normal extension-priority rules.

Since that would intentionally change existing .js wildcard behavior, I don't want to make that compatibility decision without confirmation from the team. If removing the legacy exception is the intended direction, I can update the implementation and regression tests accordingly.

@hkleungai

Jimmy Leung (hkleungai) commented Sep 29, 2026 •

Copy link
Copy Markdown

intentionally includes a .js and .d.ts file referring to the same input.

One can imagine that, if projects are running so fast, if old js source files keep growing, then adding explicit declaration files would be (one of) the most non-interuptive way for adding some flavour of typechecking. Kinda like C-style codebase, with all the .h headers and .c sources

I am intentionally skipping the inline jsdoc approach in above. In terms of typegen, for some people, jsdoc is a bit harder to be done right, than having explicit typedef files.

Not sure whether or not this coexisting pattern is becoming too uncommon in js world, but I would say in older versions of typescript, it is a possible way to do things :)

@hardikkaurani

Hardik K (hardikkaurani) commented Sep 29, 2026 •

Copy link
Copy Markdown
Author

Jimmy Leung (@hkleungai) That's a fair point. Authoring a .d.ts alongside a .js file is certainly a valid pattern, particularly for older JavaScript codebases that maintain explicit declaration files.

The distinction here, though, is between the validity of that pattern and how the compiler handles those files when they are discovered together through wildcard inclusion.

When TypeScript discovers both foo.js and foo.d.ts through the same wildcard search, automatically ingesting both can result in duplicate identifier errors because the declaration file describes the same module. As discussed in #63523, this is why the compiler's extension-priority behavior generally selects the higher-priority declaration file rather than treating both as independent sources.

If a project needs to type-check the JavaScript implementation against its declarations, it can explicitly include both files via the files array or use JSDoc-based checking. For wildcard discovery, however, keeping the existing priority-based deduplication avoids the duplicate-identifier case.

That still leaves the compatibility question around the existing .js/.d.ts legacy exception, which I think is the part that needs maintainer direction before changing the implementation.

@hkleungai

Jimmy Leung (hkleungai) commented Sep 29, 2026 •

Copy link
Copy Markdown

If a project needs to type-check the JavaScript implementation against its declarations, it can explicitly include both files via the files array or use JSDoc-based checking. For wildcard discovery, however, keeping the existing priority-based deduplication avoids the duplicate-identifier case.

Imo, "switching" to files array for js-source-ts-type mix, while staying true to include array for the remaining typescript source files, sounds like a hack, I guess (?

Or I should say, this approach of digging deep into the distinction on include array vs files array, just because there is a mix of js source and ts types in the project, sounds too mentally demanding to deal with. I don't know if I am the only one feeling that though.

Naively, in user perspective, I kinda hope I can put the all these sources as-is on include array, and let tsc does all the dirty low level processing work for me. 😕

@hardikkaurani

Hardik K (hardikkaurani) commented Sep 29, 2026 •

Copy link
Copy Markdown
Author

Jimmy Leung (@hkleungai) I agree with that. I don't think the intended fix should require users to switch from include to files just to handle a .js + .d.ts project.

I only mentioned explicit inclusion to distinguish intentional coexistence from wildcard discovery; I don't mean to suggest files as the recommended workaround.

For this PR, I think the important question is how include should consistently resolve .js/.d.ts, .mjs/.d.mts, and .cjs/.d.cts.

I'll leave the compatibility/design decision around the existing .js legacy behavior to the maintainers before changing the implementation.

@ljharb

Copy link
Copy Markdown
Contributor

Wesley Wigham (@weswigham) i intentionally include a .js and .d.ts in all of my typed npm packages, and intend to continue to do so. That's the only way to write typed JS (without writing TS) that I'm aware of.

@hardikkaurani

Hardik K (hardikkaurani) commented Sep 30, 2026 •

Copy link
Copy Markdown
Author

Jordan Harband (@ljharb) Thanks for sharing that context! It's really helpful to see concrete examples of this pattern being actively used in the ecosystem.

Wesley Wigham (@weswigham) Given Jordan's feedback that authoring a .js file alongside a companion .d.ts is an intentional and relied-upon pattern for typed JS packages, perhaps it makes sense to preserve this behavior rather than deprecate it.

If so, extending this existing behavior to .mjs and .cjs files, which is what the current PR implementation does, would ensure consistent compiler behavior across all module formats while fully supporting this authoring pattern.

Would you be open to proceeding with the current approach in light of this?

@weswigham

Copy link
Copy Markdown
Member

Given Jordan's feedback that authoring a .js file alongside a companion .d.ts is an intentional and relied-upon pattern for typed JS packages, perhaps it makes sense to preserve this behavior rather than deprecate it.

No.

intentionally include a .js and .d.ts in all of my typed npm packages, and intend to continue to do so.

Yeah, that's normal, and has no bearing on include or default wildcard file searching behavior, since it's in node_modules, and consumers are only gonna load the .d.ts files...

If you mean "I write my packages with manually authored .d.ts and .js files side-by-side in parallel trees with no emit and no declaration emit - great, you can still do that with include fixed to only pull in .d.ts, just manually load the files through a files list because it's very uncommon, I think, and 99% of the time it happens it's because of a configuration error (eg, emitting to the input dir). Plus, doing imports "right" in a project structure like that is challenge mode: impossible because basically all your .js file imports will end up actually importing the .d.ts files you wrote for their siblings, so it's more like every .js file in your project is a leaf node, not a parallel tree to the declaration files. It is almost definitely not doing what most people want, unless what they want is rather peculiar.

I'd be happier to fix include to bring .js inline with modern extensions, not break modern extensions to have the same config footguns .js extension loading has just to save you from needing to write a files list or manual ///<reference directives.

That's the only way to write typed JS (without writing TS) that I'm aware of.

We emit declaration files for jsdoc annotated js, which, as of TS7, has a much much closer correspondence to how we check and declaration emit TS files. That is the well-supported, non-footgun "how do I check my JS" path - not the weird .d.ts/.js sibling files that only override sibling imports and don't interact with file contents at all.

@hardikkaurani

Copy link
Copy Markdown
Author

Wesley Wigham (@weswigham) I've updated the implementation according to your feedback. We removed the legacy .js exception from the extension priority logic during wildcard resolution instead of extending it to modern extensions. This aligns legacy .js behavior with modern .mjs/.cjs behavior, creating a consistent and deterministic include resolution that prevents the configuration footguns. I've also updated the regression tests to assert that .js, .mjs, and .cjs implementations are consistently skipped when higher priority .d.ts, .d.mts, and .d.cts files are present.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

For Uncommitted Bug PR for untriaged, rejected, closed or missing bug

Projects

Status: Not started

Development

Successfully merging this pull request may close these issues.

checkJs skips .mjs/.cjs beside a .d.mts/.d.cts, but not .js beside a .d.ts

5 participants